[Schema][Server] Harden sampling tools against the spec - #420
Merged
chr-hertel merged 2 commits intoAug 15, 2026
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
August 15, 2026 01:36
Report tool-flow violations as -32602 instead of dropping the request, accept resource_link in tool results, gate tools on sampling.tools, and reject empty content.
chr-hertel
force-pushed
the
sampling-tools-followup
branch
from
August 15, 2026 01:45
468217f to
f19603e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens sampling tool flows to match MCP validation, capability, content, and serialization requirements.
Changes:
- Validates complete tool-use/tool-result flows and returns
-32602for violations. - Adds sampling capability helpers, content normalization APIs, and stricter schema validation.
- Expands unit/integration coverage and updates documentation and examples.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
CHANGELOG.md |
Records sampling changes and BC breaks. |
docs/client.md |
Updates sampling stop reason. |
docs/server-client-communication.md |
Documents capability gates and tool loops. |
examples/client/http_client_communication.php |
Corrects stop reason. |
examples/client/stdio_client_communication.php |
Corrects stop reason. |
src/Client/Handler/Request/SamplingRequestHandler.php |
Returns invalid-params errors for flow violations. |
src/Schema/ClientCapabilities.php |
Documents sampling sub-capabilities. |
src/Schema/Content/SamplingMessage.php |
Moves flow validation and adds block access. |
src/Schema/Content/ToolResultContent.php |
Supports resource links and omits false isError. |
src/Schema/Request/CreateSamplingMessageRequest.php |
Implements tool-flow validation. |
src/Schema/Result/CreateSamplingMessageResult.php |
Enforces role/content constraints and block access. |
src/Server/ClientGateway.php |
Validates requests and exposes capability checks. |
tests/Integration/Fixture/sampling_tools.php |
Adds sampling-tool server fixture. |
tests/Integration/SamplingToolsTest.php |
Covers complete tool loops and capability gating. |
tests/Unit/Client/Handler/Request/SamplingRequestHandlerTest.php |
Covers handler validation responses. |
tests/Unit/Schema/ClientCapabilitiesSamplingTest.php |
Removes superseded capability tests. |
tests/Unit/Schema/ClientCapabilitiesTest.php |
Consolidates sampling capability tests. |
tests/Unit/Schema/Content/SamplingMessageTest.php |
Covers message hydration and validation. |
tests/Unit/Schema/Content/SamplingToolContentTest.php |
Removes superseded combined tests. |
tests/Unit/Schema/Content/ToolResultContentTest.php |
Covers tool-result schema behavior. |
tests/Unit/Schema/Content/ToolUseContentTest.php |
Covers tool-use serialization and validation. |
tests/Unit/Schema/Request/CreateSamplingMessageRequestTest.php |
Covers valid and invalid tool flows. |
tests/Unit/Schema/Result/CreateSamplingMessageResultTest.php |
Covers result constraints and shapes. |
tests/Unit/Schema/ToolChoiceTest.php |
Covers tool-choice modes and validation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
| if (Role::Assistant === $role && $item instanceof ToolResultContent) { | ||
| throw new InvalidArgumentException('ToolResultContent is only valid in user sampling messages.'); | ||
| if (\is_array($content)) { |
| ) { | ||
| foreach ($content as $item) { | ||
| if (!$item instanceof TextContent && !$item instanceof ImageContent && !$item instanceof AudioContent && !$item instanceof EmbeddedResource) { | ||
| if (!$item instanceof TextContent && !$item instanceof ImageContent && !$item instanceof AudioContent && !$item instanceof ResourceLink && !$item instanceof EmbeddedResource) { |
Comment on lines
+51
to
61
| if (\is_array($content)) { | ||
| if ([] === $content) { | ||
| throw new InvalidArgumentException('CreateSamplingMessageResult content must not be empty.'); | ||
| } | ||
|
|
||
| foreach ($content as $item) { | ||
| if (!$item instanceof TextContent && !$item instanceof ImageContent && !$item instanceof AudioContent && !$item instanceof ToolUseContent) { | ||
| throw new InvalidArgumentException('CreateSamplingMessageResult contains an unsupported content block.'); | ||
| } | ||
| } | ||
| } |
array_filter() and friends preserve keys, and a keyed array serializes as a JSON object instead of the content-block array the schema requires.
chr-hertel
added a commit
to chr-hertel/php-sdk
that referenced
this pull request
Aug 15, 2026
Ports the type definitions the 2026-07-28 revision introduces outside of sampling tool use, which modelcontextprotocol#409 and modelcontextprotocol#420 already cover. Every addition is optional and defaults to current behaviour, so a connection negotiated on an older revision is unaffected. Elicitation gains modes. ElicitationMode splits `form` — build a form from the requested schema — from `url`, which sends the user out of band and returns only the accept/decline/cancel outcome. That is why requestedSchema becomes optional and `url` appears beside it. ClientCapabilities learns the matching sub-capabilities, where an `elicitation` naming no mode declares form, the only shape that existed before url mode. Schemas loosen where the revision loosens them: Tool::outputSchema may describe any JSON value rather than only an object, and CallToolResult::structuredContent follows. Adds the three error codes the revision defines (-32020 header mismatch, -32021 missing required client capability, -32022 unsupported protocol version) and switches ProtocolVersionMiddleware to the last of them, so a rejected version carries the supported set as structured data the client can retry from rather than only as prose. Icon gains `theme`, Implementation gains `title`.
chr-hertel
added a commit
to chr-hertel/php-sdk
that referenced
this pull request
Aug 15, 2026
…-28 surface Ports the type definitions this SDK still misses outside of sampling tool use, which modelcontextprotocol#409 and modelcontextprotocol#420 already cover. Every addition is optional and defaults to current behaviour, so a connection negotiated on an older revision is unaffected. From 2025-11-25, elicitation gains modes. ElicitationMode splits `form` — build a form from the requested schema — from `url`, which sends the user out of band and returns only the accept/decline/cancel outcome. That is why requestedSchema becomes optional and `url` appears beside it. ClientCapabilities learns the matching sub-capabilities, where an `elicitation` naming no mode declares form, the only shape that existed before. Icon gains `theme` from the same revision, and Implementation gains the `title` BaseMetadata has carried since 2025-06-18. From 2026-07-28, schemas loosen where the revision loosens them: SEP-2106 drops the object-only restriction, so Tool::outputSchema may describe any JSON value and CallToolResult::structuredContent follows. The same revision defines three error codes (-32020 header mismatch, -32021 missing required client capability, -32022 unsupported protocol version). ProtocolVersionMiddleware switches to the last of them, so a rejected version carries the supported set as structured data the client can retry from rather than only as prose.
chr-hertel
added a commit
that referenced
this pull request
Aug 17, 2026
…-28 surface (#421) * [Schema][Server] Close the 2025-11-25 schema gaps and add the 2026-07-28 surface Ports the type definitions this SDK still misses outside of sampling tool use, which #409 and #420 already cover. Every addition is optional and defaults to current behaviour, so a connection negotiated on an older revision is unaffected. From 2025-11-25, elicitation gains modes. ElicitationMode splits `form` — build a form from the requested schema — from `url`, which sends the user out of band and returns only the accept/decline/cancel outcome. That is why requestedSchema becomes optional and `url` appears beside it. ClientCapabilities learns the matching sub-capabilities, where an `elicitation` naming no mode declares form, the only shape that existed before. Icon gains `theme` from the same revision, and Implementation gains the `title` BaseMetadata has carried since 2025-06-18. From 2026-07-28, schemas loosen where the revision loosens them: SEP-2106 drops the object-only restriction, so Tool::outputSchema may describe any JSON value and CallToolResult::structuredContent follows. The same revision defines three error codes (-32020 header mismatch, -32021 missing required client capability, -32022 unsupported protocol version). ProtocolVersionMiddleware switches to the last of them, so a rejected version carries the supported set as structured data the client can retry from rather than only as prose. * [Schema] Reject what the 2026-07-28 types cannot represent `Implementation::title` reached the typed constructor unchecked, so malformed wire data raised a TypeError instead of InvalidArgumentException. `ToolUseContent::input` accepted a list and serialized it as a JSON array, where the protocol requires an object. The empty array stays exempt: it is also an empty map and still emits `{}`. `ToolChoice` and `ElicitRequest` read their mode with isset(), which is false for an explicit null, so `{"mode": null}` silently became the default instead of being rejected. Both use array_key_exists() now, letting the existing type check refuse null. * [Client][Server] Advertise the implementation title through both builders `Implementation::title` could be parsed but never sent: neither `Client\Builder::setClientInfo()` nor `Server\Builder::setServerInfo()` accepted one, so every SDK user emitted null. Both gain a trailing optional `$title`. On the server it sits where the Implementation constructor already puts it, so existing positional calls keep their meaning; the client builder forwards it by name, leaving the icons and websiteUrl slots defaulted. * [Schema] Serialize every non-null structuredContent The object-only hydration guard was never that: `!is_array()` admitted `[1, 2, 3]` and `[]`, which serialize to JSON arrays, while rejecting the scalars 2026-07-28 permits. The truthiness emission gate was backwards in the same way — it dropped `[]`, `0`, `false` and `""`, yet emitted lists, strings and an empty stdClass. Hydration now accepts any JSON value, and `null` alone means absent, matching `ToolResultContent` which already carries this field that way. Which values a given revision permits is a question for version-aware serialization, which results cannot answer yet. * [Schema][Server] Make url elicitation reachable `ElicitRequest::forUrl()` built a request no SDK user could send: the only public gateway method always constructed form mode from an ElicitationSchema, and `request()` is private. `elicitUrl()` joins `elicit()`, and both funnel through one send path that hydrates the result with the request's own mode. Without that, a url-mode accept — contentless by design — threw, because ElicitResult requires content whenever the action is accept. The result carries no discriminator of its own, so the mode has to come from the request it answers. `supportsElicitationUrl()` reports whether the client named the mode, reusing the sub-capability reader the sampling checks already use. * [Schema] Add CHANGELOG entry for the 2026-07-28 surface * [Capability][Schema][Server] Carry the widened structuredContent end to end Review follow-up on the SEP-2106 widening, which stopped at the type. `ToolReference::extractStructuredContent()` returned `?array`, so a tool declaring a scalar `outputSchema` had its result dropped and logged as unsendable. It returns `mixed` now and keeps a scalar — but only from 2026-07-28 on, and only when the tool declared an outputSchema: without one the value is already carried in `content`, and advertising it twice is not an improvement. `CallToolHandler` only warned about list-shaped `structuredContent` on a self-built `CallToolResult`, so a scalar reached revisions that require an object unremarked. The check is on the shape now, not on the list case alone. `ElicitResult` retained a `content` the spec says is absent from url-mode results, leaving a malformed response indistinguishable from a valid one. `Tool::jsonSerialize()` emitted the empty root schema as `[]`, which is not a schema at all. Sub-schemas already had this treatment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #409, now rebased onto
mainso the diff is only this work.Fixes
SamplingMessage::__construct, so they fired during hydration —MessageFactoryreturns the resulting exception inside the message array and the client'sprocessMessage()loop ignores anything that is not aResponse|Error|Request|Notification. A violating request got no reply at all and the server blocked until timeout. They now live inCreateSamplingMessageRequest::validateToolFlow(), which the client handler runs to return-32602and the gateway runs to fail before sending.ToolUseContentmust be answered by a matchingToolResultContentbefore the conversation continues.resource_linkis accepted in tool results, matchingCallToolResult.contentas the spec requires.ClientGateway::supportsSamplingTools()/supportsSamplingContext(), matchingsupportsSampling(). Servers must not sendtoolsto a client that did not advertisesampling.tools, and there was no way to check.SamplingMessageandCreateSamplingMessageResult.isErroris omitted when false, per its schema default.Cleanups
getContentBlocks()onSamplingMessageandCreateSamplingMessageResultso callers can iterate without branching on the single-vs-list union.ToolResultContent::$contentclaimedContent[]but rejects most of it;SamplingMessageDatadescribed the wire shape while typingjsonSerialize().$content, assistant-only result role).end_turn, which is not a spec value.Tests
Per-class unit tests for
ToolUseContent,ToolResultContent,SamplingMessageandToolChoice, validator coverage on the request, handler coverage for the-32602path, and an integration test running a full multi-turn tool loop plus the capability gate.1219 tests pass, PHPStan clean, cs-fixer clean.